Skip to content

RoPE kernel for latency test: issue #166 (issue #94), standalone 8-core (4x2) - #194

Merged
lasch merged 3 commits into
torch-spyre:mainfrom
yuhaohaoyu:issue-94-rope-create
Aug 18, 2026
Merged

RoPE kernel for latency test: issue #166 (issue #94), standalone 8-core (4x2)#194
lasch merged 3 commits into
torch-spyre:mainfrom
yuhaohaoyu:issue-94-rope-create

Conversation

@yuhaohaoyu

Copy link
Copy Markdown
Collaborator

Issues addressiog

What's done?

Add standalone 8-core (4x2) RoPE kernel (issue #166) aligning with Granite-8b scale

  • Grid [4,2], TILE_SEQ=256: head loop (20 iter) × seq-tile loop (4 iter).
  • Half-layout RoPE (LLaMA convention): y[0:D/2] = x[0:D/2]*cos - x[D/2:D]*sin; memory-bound at AI ~0.63 FLOPs/byte.
  • Tests: correctness, latency scaling (memory/simd/comm), MLIR frontend adapt.

@yuhaohaoyu yuhaohaoyu self-assigned this Aug 3, 2026
@yuhaohaoyu yuhaohaoyu added the documentation Improvements or additions to documentation label Aug 3, 2026
@yuhaohaoyu yuhaohaoyu moved this from Backlog to In review in Torch-Spyre Device Enablement Aug 3, 2026
…ing with Granite-8b scale

- Grid [4,2], TILE_SEQ=256: seq-tile loop (4 iter) - head loop (20 iter).
- Half-layout RoPE (LLaMA convention): y[0:D/2] = x[0:D/2]*cos - x[D/2:D]*sin; memory-bound at AI ~0.73 FLOPs/byte.
- Tests: correctness, latency scaling (memory/simd/comm), MLIR frontend adapt.

Signed-off-by: Hao Yu <yuh@us.ibm.com>
@yuhaohaoyu
yuhaohaoyu force-pushed the issue-94-rope-create branch from a2b64bd to c9a3e26 Compare August 5, 2026 16:05
@yuhaohaoyu

yuhaohaoyu commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Notes on addressing a loop-interchange rewriting of the standalone RMSNorm kernel.

Lars left a comment somewhere about a potential poor-forming of the mlir file.
"
Claude has a performance suggestion about your PR 194:

The loop nest is head-outer (20 iterations) / seq-tile-inner (4 iterations). `cos`/`sin`
depend only on seq position, not on head — but because head is the outer loop, each
`[256, 64]` cos/sin tile gets reloaded once per `(head, seq-tile)` pair, i.e. 80 loads total,
instead of once per seq-tile (4 loads total) if the loop nest were flipped (seq-tile outer,
head inner), which would let one cos/sin load serve all 20 heads for that tile — a 20×
reduction in cos/sin HBM traffic with no algorithmic change.

Does that have any grounding in reality?
"


In Gist: It translates that 'loop interchange' is the obvious path to avoid unnecessary reload sin-cos constants for RMSNorm.


Actions: update the kernel with following design thoughts:

interchanging those 2 loops is convincing now

  • now: loop-over attention-heads; loop-over sequence-blocks; (re)load sin/cos; ... endloop; endloop
  • interchanged: loop-over seq-blocks; (re)load sin/cos; loop-over attn-blocks; ... endloop; endloop.

I wrongfully assumed that MLIR has a way to pass part of the array to inside the loop. There was 'preload the needed sin/cos to my core' outside the loop nest. Yet MLIR can not link that load to the 'slicing-relayout/reload' of cos/sin.

Now will follow your suggestion,
1. remove the preload, (not useful when we have the current inner-loop not inlined.)
2. interchange loop
3. hoist the sin/cos load out of the inner loop.

@lasch lasch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Optional item:

G1 (minor, cheap). test_rope_fwd_zero_input provides little independent signal: with x = 0, both y_first and y_second reduce to 0 * cos ± 0 * sin = 0 regardless of whether cos/sin are loaded or indexed correctly, so this test would still pass even if the cos/sin addressing were completely broken. The main test_rope_fwd_correctness test already covers the general case well via random input compared against a full-tensor NumPy reference across all 8 cores, so this isn't a large gap — but a more targeted edge case (e.g. an input that isolates the sin/cos cross-term, or a boundary-head/boundary-tile-specific check) would add more real signal than the zero-input check does today.

@lasch
lasch merged commit e16ae78 into torch-spyre:main Aug 18, 2026
2 checks passed
fabianlim added a commit to fabianlim/ktir-cpu that referenced this pull request Aug 18, 2026
The RoPE kernel (torch-spyre#194) landed on main after this branch renamed
`#ktdp.spyre_memory_space<HBM>` to `#ktdp.memory_space<global>`, so the merge
of main brought in four occurrences of the old spelling. It no longer parses
under the bumped ktir-mlir-frontend pin, which is why the frontend job failed
on the merge commit while the regex-parser job passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yu Chin Fabian Lim <flim@sg.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

create rope_fwd_ktir.mlir

3 participants